Merged
Conversation
Contributor
Greptile SummaryThis PR fixes a flash of skeleton UI during SSR by (1) calling
Confidence Score: 4/5Safe to merge for the skeleton fix, but the reloading opacity regression should be addressed before landing. The vendor/ change is correct and focused. The SignedIn.svelte change achieves its goal (no skeleton flash on SSR) but introduces a P1 UX regression: filter-change reloads no longer show any visual feedback for the majority of users who arrive via SSR. app/javascript/pages/Home/SignedIn.svelte — the reloading overlay logic needs to be preserved or re-implemented for the SSR render path. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Page Request] --> B{SSR?}
B -- Yes --> C[App.svelte: setPage sync]
C --> D{dashboard_stats in props?}
B -- No / Client Nav --> D
D -- Yes --> E[Render dashboard content directly\nno skeleton, no reloading overlay]
D -- No --> F[Render Deferred component]
F --> G{Data loaded?}
G -- No --> H[Show Skeleton fallback]
G -- Yes --> I[Render dashboard content\nwith reloading opacity support]
E --> J[User changes filters]
J --> K[refreshDashboardData via router.visit]
K --> L[dashboard_stats updated\nbut NO loading indicator shown]
I --> J
Prompt To Fix All With AIThis is a comment left during a code review.
Path: app/javascript/pages/Home/SignedIn.svelte
Line: 136-209
Comment:
**Reloading opacity lost in SSR branch**
When `dashboard_stats` is already populated on SSR/hydration the component permanently stays in the `{#if dashboard_stats}` branch. That branch has no `reloading` indicator, so when `refreshDashboardData` triggers a partial Inertia visit (`only: ["dashboard_stats"]`), the old data stays visible with zero visual feedback — the `class:opacity-60={reloading}` that was in the `<Deferred>` children snippet is effectively unreachable for any user who arrives via SSR (which is the common path after this change).
Consider forwarding the `reloading` state from the `<Deferred>` snippet into a shared variable, or wrapping the SSR branch in the same `<Deferred>` and skipping the fallback skeleton conditionally.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: app/javascript/pages/Home/SignedIn.svelte
Line: 136-166
Comment:
**Duplicated dashboard rendering logic**
The `TodaySentence`, `Dashboard`, and `ActivityGraph` rendering is now copy-pasted verbatim between lines 137–166 (SSR branch) and lines 179–207 (Deferred children snippet). Per the project style guide, duplicated markup like this should be extracted into a shared Svelte snippet or child component. A simple `{#snippet dashboardContent(reloading)}` declared once above the `{#if}` block would eliminate both copies and make the `reloading` opacity easy to restore.
**Context Used:** In the Hackatime repo, please ensure that the PR i... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "Don't show skeleton on SSR!" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent dashboard skeleton UI from rendering during SSR by ensuring deferred content can resolve against real page props, and by adjusting the Home/SignedIn dashboard rendering logic.
Changes:
- Pre-populate Inertia’s global
pagestore before the initial render in the Svelte adapterApp.svelteto support SSR resolution forDeferred/WhenVisible. - Update
Home/SignedIn.svelteto render dashboard content immediately whendashboard_statsis already present, otherwise fall back to<Deferred>with skeletons.
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| vendor/inertia/packages/svelte/dist/components/App.svelte | Sets the global page store earlier to affect SSR behavior for deferred content. |
| app/javascript/pages/Home/SignedIn.svelte | Avoids skeleton rendering when dashboard_stats is present by bypassing <Deferred> in that case. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.